Skip to content

Comments

Feature/subfolders in outputs#21

Merged
Alon-Alexander merged 3 commits intomainfrom
feature/subfolders_in_outputs
Feb 10, 2026
Merged

Feature/subfolders in outputs#21
Alon-Alexander merged 3 commits intomainfrom
feature/subfolders_in_outputs

Conversation

@Alon-Alexander
Copy link
Owner

@Alon-Alexander Alon-Alexander commented Feb 10, 2026

User description

Solves #19


PR Type

Enhancement


Description

  • Add subfolder support to PMAnalysis$get_output_path() method

  • Enable both unix-style (/) and windows-style () path delimiters

  • Update list_outputs() to handle nested folder structures

  • Extend get_artifact() to support subfolder paths in both PMAnalysis and PMProject

  • Add comprehensive test coverage for nested output directories


Diagram Walkthrough

flowchart LR
  A["get_output_path<br/>with subfolders"] -->|"normalize path<br/>delimiters"| B["Create nested<br/>directories"]
  B -->|"recursive=TRUE"| C["list_outputs<br/>finds nested files"]
  A -->|"support both / and \"| D["get_artifact<br/>finds files<br/>in subfolders"]
  C -->|"return PMData<br/>objects"| E["Read/Write<br/>nested outputs"]
Loading

File Walkthrough

Relevant files
Enhancement
3 files
analysis.R
Add subfolder support to get_output_path and list_outputs
+13/-14 
data.R
Auto-create parent directories on write operations             
+5/-2     
project.R
Normalize subfolder IDs in get_artifact method                     
+4/-0     
Tests
1 files
test_analysis.R
Add comprehensive tests for nested folder operations         
+76/-0   
Configuration changes
2 files
DESCRIPTION
Bump version from 0.1.10 to 0.1.11                                             
+1/-1     
pkgdown.yml
Update build timestamp and metadata                                           
+1/-1     
Documentation
7 files
NEWS.md
Document new subfolder support features                                   
+8/-0     
index.md
Add changelog entry for version 0.1.11                                     
+9/-0     
getting-started.md
Update documentation with new subfolder examples                 
+11/-11 
PMAnalysis.md
Document subfolder support in get_output_path                       
+19/-5   
PMAnalysis.Rd
Update roxygen documentation for subfolder feature             
+11/-0   
getting-started.html
Regenerate HTML documentation with updates                             
+13/-13 
PMAnalysis.html
Regenerate reference documentation HTML                                   
+20/-7   
Additional files
41 files
data_utils.R +0/-1     
404.html +1/-1     
LICENSE.html +1/-1     
file-formats.html +3/-3     
file-formats.md +1/-1     
index.html +1/-1     
input-definitions.html +2/-2     
slurm-integration.html +2/-2     
authors.html +3/-3     
authors.md +2/-2     
index.html +1/-1     
index.html +12/-1   
PMData.html +1/-1     
PMProject.html +6/-6     
PMProject.md +5/-5     
PMSlurmRun.html +1/-1     
dot-cancel_slurm_job.html +1/-1     
dot-check_missing_entries.html +1/-1     
dot-check_missing_files.html +1/-1     
dot-check_slurm_job_done.html +1/-1     
dot-check_slurm_job_success.html +1/-1     
dot-extract_input_ids.html +1/-1     
dot-find_code_folder_name.html +1/-1     
dot-format_validation_error.html +1/-1     
dot-get_slurm_job_error.html +1/-1     
dot-submit_slurm_job_with_env.html +1/-1     
dot-validate_input_fields.html +1/-1     
dot-validate_input_files.html +1/-1     
dot-validate_inputs_schema.html +1/-1     
dot-validate_local_inputs_schema.html +1/-1     
index.html +1/-1     
is_slurm_available.html +1/-1     
pm_create_project.html +2/-2     
pm_create_project.md +1/-1     
pm_infer_analysis.html +7/-7     
pm_infer_analysis.md +6/-6     
pm_project.html +2/-2     
pm_project.md +1/-1     
pm_read_file.html +1/-1     
pm_write_file.html +1/-1     
search.json +1/-1     

@Alon-Alexander Alon-Alexander self-assigned this Feb 10, 2026
@qodo-free-for-open-source-projects
Copy link

qodo-free-for-open-source-projects bot commented Feb 10, 2026

PR Code Suggestions ✨

  • Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

Previous suggestions

Suggestions up to commit 40bb8e2
CategorySuggestion                                                                                                                                    Impact
Possible issue
Filter out directories from results

In the list_outputs method, filter the results of list.files to exclude
directories, ensuring only files are returned as PMData objects.

R/analysis.R [243-255]

 # Get all files in the directory
 files <- list.files(outputs_dir, full.names = FALSE, recursive = TRUE)
+if (length(files) == 0) {
+  return(list())
+}
+
+# Filter to only files (not directories)
+full_paths <- file.path(outputs_dir, files)
+is_file <- !dir.exists(full_paths)
+files <- files[is_file]
+
 if (length(files) == 0) {
   return(list())
 }
 
 # Create PMData objects using lapply
 file_ids <- gsub("\\", "/", tools::file_path_sans_ext(files), fixed = TRUE)
 file_paths <- normalizePath(file.path(outputs_dir, files), mustWork = FALSE)
 
 lapply(seq_along(files), function(i) {
   PMData$new(id = file_ids[i], path = file_paths[i])
 })
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a bug where directories would be listed as outputs, and provides a valid fix to filter them out.

Medium
Prevent writing to an existing directory

In the write method, add a check to prevent writing if self$path is an existing
directory, and stop with an informative error message.

R/data.R [103-115]

 write = function(x, ...) {
+  # Stop if path is an existing directory
+  if (dir.exists(self$path)) {
+    stop("Cannot write to a path that is an existing directory: ", self$path, call. = FALSE)
+  }
+
   # Create the folder in case it doesn't exist
   dir.create(dirname(self$path), showWarnings = FALSE, recursive = TRUE)
 
   # For RData files, we need   to preserve object names from the original call
   ext <- tolower(tools::file_ext(self$path))
   if (ext %in% c("rdata", "rda")) {
     # ...
   } else {
     # Call pm_write_file for other file types
     pm_write_file(self$path, x, ...)
   }
   invisible(self)
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies an edge case where writing to a path that is an existing directory would fail, and proposes a reasonable guard clause.

Medium

@codecov
Copy link

codecov bot commented Feb 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (68.13%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Files with missing lines Coverage Δ
R/analysis.R 54.51% <100.00%> (-0.44%) ⬇️
R/data.R 84.05% <100.00%> (+0.47%) ⬆️
R/data_utils.R 95.65% <ø> (ø)
R/project.R 94.02% <100.00%> (+0.04%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Repository owner deleted a comment from qodo-free-for-open-source-projects bot Feb 10, 2026
@qodo-free-for-open-source-projects

Persistent suggestions updated to latest commit e664cf8

@Alon-Alexander Alon-Alexander merged commit 9068984 into main Feb 10, 2026
12 of 13 checks passed
@Alon-Alexander Alon-Alexander deleted the feature/subfolders_in_outputs branch February 10, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant